-
Notifications
You must be signed in to change notification settings - Fork 4.9k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Optimize memory allocations in the event processing pipeline #36830
Conversation
💔 Build Failed
Expand to view the summary
Build stats
Pipeline error
❕ Flaky test reportNo test was executed to be analysed. 🤖 GitHub commentsExpand to view the GitHub comments
To re-run your PR in the CI, just comment with:
|
19dc9e5
to
30965cf
Compare
Previously, every time a processor ran on an event we made a clone of the entire event for two reasons: 1. this event could have some nested maps that are shared among multiple events. 2. in case a processor fails to make a change it should be able to revert its partial changes. This change added a new `EventEditor` wrapper that is used for collecting pending event changes in processors with an option to `Apply` or `Reset` them. Additionally, this `EventEditor` takes care of the efficient memory management when making changes to an event by cloning only the nested maps that processors access or modify. Most of the processors just put new keys or delete existing keys on the root-level, so most of the time the nested maps in the event remain untouched and it does not require the whole event to be cloned.
7cdf193
to
8c636f6
Compare
can't wait for this to land. will have a great positive impact on performance! |
8c636f6
to
b236659
Compare
Closing in favour of #36958 |
decided to first deliver a first set of processors? |
Previously, every time a processor ran on an event we made a clone of the entire event for two reasons:
this event could have some nested maps that are shared among multiple events.
in case a processor fails to make a change it should be able to revert its partial changes.
This change added a new
EventEditor
wrapper that is used for collecting pending event changes in processors with an option toApply
orReset
them.Additionally, this
EventEditor
takes care of the efficient memory management when making changes to an event by cloning only the nested maps that processors access or modify. Most of the processors just put new keys or delete existing keys on the root-level, so most of the time the nested maps in the event remain untouched and it does not require the whole event to be cloned.Processors migration progress:
BEFORE/AFTER benchmark results will be here once PR is finished
Checklist
- [ ] I have made corresponding changes to the documentation- [ ] I have made corresponding change to the default configuration filesCHANGELOG.next.asciidoc
orCHANGELOG-developer.next.asciidoc
.How to test this PR locally
I added unit tests and a benchmark.
Related issues